Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make annotations on expect declarations comply with new compiler restriction #3815

Merged
merged 2 commits into from
Jul 21, 2023

Conversation

merfemor
Copy link
Contributor

In KT-58551 we require all annotations from expect declaration to be present on actual.

This commit fixes the following violations:

  1. DefaultDelay has @PublishedApi only on expect. But it is not used from public inline functions, so we can safely remove annotation.

  2. expect IgnoreJreRequirement has AnnotationTarget.TYPE which is not present in actual. This is because Java target TYPE doesn't correspond to Kotlin target TYPE. Since IgnoreJreRequirement is internal, we can safely change targets on expect and make them equal to actual.

@merfemor merfemor requested a review from qwwdfsad July 19, 2023 11:22
@merfemor merfemor added the kotlin-merge-blocker PRs labeled as merge-blockers are effectively preventing a corresponding merge into main Kotlin repo label Jul 19, 2023
@qwwdfsad qwwdfsad requested a review from mshishkina July 19, 2023 13:21
@merfemor merfemor force-pushed the roman.efremov/MR/expect-annotations branch from 3c0e4d1 to 45f749d Compare July 19, 2023 13:53
@merfemor merfemor requested a review from qwwdfsad July 19, 2023 13:54
@merfemor merfemor force-pushed the roman.efremov/MR/expect-annotations branch from 45f749d to 90542df Compare July 19, 2023 15:24
Copy link
Member

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please merge the suggestions and it's good to go

…riction

In KT-58551 we require all annotations from expect declaration to
be present on actual.

This commit fixes the following violations:

1. `DefaultDelay` has `@PublishedApi` only on expect, thus it must be copied to
all actuals.

2. `unwrap` has the same problem.

3. expect `IgnoreJreRequirement` has `AnnotationTarget.TYPE` which is not present in actual.
This is because Java target TYPE doesn't correspond to Kotlin target TYPE.
Since `IgnoreJreRequirement` is internal, we can safely change targets on expect and make
them equal to actual.
@merfemor merfemor force-pushed the roman.efremov/MR/expect-annotations branch from 7e25e7e to 3c9e856 Compare July 21, 2023 11:05
@qwwdfsad qwwdfsad self-requested a review July 21, 2023 12:30
@merfemor merfemor merged commit 387628b into develop Jul 21, 2023
@merfemor merfemor deleted the roman.efremov/MR/expect-annotations branch July 21, 2023 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kotlin-merge-blocker PRs labeled as merge-blockers are effectively preventing a corresponding merge into main Kotlin repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants